-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(auth): handle ENOTDIR when opening cert config #10697
fix(auth): handle ENOTDIR when opening cert config #10697
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see feedback on the linked issue.
b4f7650
to
eb7bb72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR and the test coverage 🎆
934674d
to
a60add3
Compare
Failing test:
|
Oh, right! I'll update it to skip that test when the UID is |
Some users explicitly have their home directories set to `/dev/null`. When opening a file that's "under that directory", instead of getting the normal `ENOENT`, one should expect to get `ENOTDIR`. Unfortunately, due to the variety of cases under which `ENOTDIR` is returned, the go team has declined to include ENOTDIR in `ErrNotExist`. (https://golang.org/issues/18974) The most robust solution in this case (following [1]) is to treat any read error as `errSourceUnavailable`. This has the mixed-benefit of also covering EPERM. (hopefully this doesn't make it too hard to track down permissions problems for secure connect users) Resolves: googleapis#10696 [1]: https://go-review.googlesource.com/c/oauth2/+/493695
e26a384
to
10b2435
Compare
Ok, I've rebased and pushed a guard to skip the EPERM test when the UID is 0. |
Some users explicitly have their home directories set to
/dev/null
. When opening a file that's "under that directory", instead of getting the normalENOENT
, one should expect to getENOTDIR
.Unfortunately, due to the variety of cases under which
ENOTDIR
is returned, the go team has declined to include ENOTDIR inErrNotExist
. (https://golang.org/issues/18974)The most robust solution in this case (following 1) is to treat any
read error as
errSourceUnavailable
. This has the mixed-benefit of alsocovering EPERM. (hopefully this doesn't make it too hard to track down
permissions problems for secure connect users)
Resolves: #10696